Create placeholder uncertain locations modal - #1133
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1133 +/- ##
==========================================
+ Coverage 52.30% 52.36% +0.05%
==========================================
Files 354 355 +1
Lines 12091 12102 +11
Branches 2185 2188 +3
==========================================
+ Hits 6324 6337 +13
Misses 5543 5543
+ Partials 224 222 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a placeholder “Uncertain locations” dialog and uses the uncertain-locations feature flag to switch location-editing entry points from the existing LocationPickerComponent to the new placeholder modal.
Changes:
- Introduced
UncertainLocationPickerComponent(template/styles/spec) as a placeholder modal for the upcoming uncertain-locations UX. - Updated
EditService.openLocationDialogto choose between the existing and placeholder location dialogs based on theuncertain-locationsfeature flag. - Updated
ProfileEditComponent.chooseLocationForItemto use the same feature-flag-based dialog selection, with accompanying unit test updates.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/file-browser/file-browser-components.module.ts | Declares/exports the new uncertain location picker component so it’s usable in dialogs. |
| src/app/file-browser/components/uncertain-location-picker/uncertain-location-picker.component.ts | Adds the placeholder dialog component logic (reads dialog data, supports closing). |
| src/app/file-browser/components/uncertain-location-picker/uncertain-location-picker.component.html | Implements the placeholder dialog UI (header/body/footer). |
| src/app/file-browser/components/uncertain-location-picker/uncertain-location-picker.component.scss | Adds dialog layout/styling consistent with CDK-opened dialogs. |
| src/app/file-browser/components/uncertain-location-picker/uncertain-location-picker.component.spec.ts | Adds unit tests for rendering and close behaviors. |
| src/app/core/services/edit/edit.service.ts | Switches the location dialog component via FeatureFlagService.isEnabled('uncertain-locations'). |
| src/app/core/services/edit/edit.service.spec.ts | Adds/updates tests to validate dialog selection for flag on/off. |
| src/app/core/components/profile-edit/profile-edit.component.ts | Switches profile item location dialog via the same feature flag. |
| src/app/core/components/profile-edit/profile-edit.component.spec.ts | Adds test coverage for flag on/off behavior in profile edit. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
aasandei-vsp
left a comment
aasandei-vsp
left a comment
There was a problem hiding this comment.
LGTM, just a small comment.
I'd argue we would add another level of abstractisation, like a specific service for opening the uncertain location picker component, but that might be needed in the future, for now this is ok.
| ); | ||
| }); | ||
|
|
||
| it('should open UncertainLocationPickerComponent when the uncertain-locations flag is enabled', async () => { |
There was a problem hiding this comment.
toHaveBeenCalledWith satisfies any spy that has been called at some point, with the correct dialog, but this this needs to be made specific for every test.
We should add mockDialogService.open.calls.reset() to the beforeEach in the profile-edit spec, and switch all four to toHaveBeenCalledOnceWith(...). This way, we make sure every call to the mock dialog service is done specifically for every test.
This modal does not actually implement the new location editing UX, however it does set up the groundwork for that implementation by creating a new component which lives behind a feature flag. This modal is intended to replace *all* location editor modal instances, so not just record / folder but also profile milestones. Issue #1122 Use the uncertain-locations feature flag Claude-Session: https://claude.ai/code/session_01VkPycbEMFF8FJ6ZDvdebct
f0b861a to
9710b23
Compare
This PR adds a new modal which will be developed to contain the uncertain locations UX. The new modal is rendered instead of the current location editing modal depending on whether the
uncertain-locationsfeature flag has been enabled.This modal is intended to override in ALL location editing entry points (e.g. record / folder / profile).
You can test the PR's functionality by enabling the
uncertain-locationsfeature flag in your local environment and then clicking the "Click to set location" side pane in the file viewer.You'll see something like this:
I do not believe this PR will need QA since at this point it is all behind a feature flag and UX is not really being implemented.
Resolves #1122